Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to 1.19.2 #19

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Port to 1.19.2 #19

wants to merge 9 commits into from

Conversation

unilock
Copy link

@unilock unilock commented Aug 15, 2023

This PR ports the mod to Minecraft 1.19.2.

Breaking changes:

  • The chat_capture event now returns username, message, id, capture instead of message, capture, username, id, for consistency.
  • ChatMixin has been replaced with an event listener for Fabric API's ServerMessageEvents.ALLOW_CHAT_MESSAGE.

Due to changes to the way chat messages are handled and verified as of Minecraft 1.19.1, cancelling the chat message is no longer (easily) possible. Thus, fully "capturing" chat messages has been temporarily removed - other players will now see the bound player's messages regardless of whether they've been captured.

The "proper" way to fix this, in my opinion, would be to mixin to the client's ChatScreen$sendMessage, send a custom packet to the server containing the relevant data, and cancel the chat message client-side. An example implementation of this can be found in Botania:
https://github.com/VazkiiMods/Botania/blob/1.20.x/Fabric/src/main/java/vazkii/botania/fabric/mixin/client/ChatScreenMixin.java
https://github.com/VazkiiMods/Botania/blob/1.20.x/Xplat/src/main/java/vazkii/botania/common/block/block_entity/corporea/CorporeaIndexBlockEntity.java#L380

I did not implement this fix myself, as I am lazy. Hence why this PR is a draft!

@hugeblank
Copy link
Owner

Thanks in advance for the PR!

I knew chat message handling was going to be tricky, thank you for addressing it and mentioning a potential fix. Such a bummer that Mojang decided to implement it the way they did :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants